Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/0.2.0 #103

Merged
merged 56 commits into from
Apr 5, 2024
Merged

Feature/0.2.0 #103

merged 56 commits into from
Apr 5, 2024

Conversation

@mgcth mgcth self-assigned this Mar 23, 2024
@docNord
Copy link
Collaborator

docNord commented Mar 29, 2024

Are we returning pandas dfs across the board now?

@mgcth
Copy link
Collaborator Author

mgcth commented Mar 29, 2024

Are we returning pandas dfs across the board now?

We are returning pydantic models, which usually contain a dataframe that is validated with pandera.

Copy link
Collaborator

@docNord docNord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, I really appreciate the data structure and the setup, it is neat!

I might have appreciated a datetime format rather than string dates (datetime.datetime.strptime(data.period["Tidsperiod (fr.o.m)"][0],"%Y-%m-%d %H:%M:%S"), but we can leave that parsing for the parent class, if needed.

Again, awesome!

@mgcth
Copy link
Collaborator Author

mgcth commented Apr 4, 2024

I might have appreciated a datetime format rather than string dates (datetime.datetime.strptime(data.period["Tidsperiod (fr.o.m)"][0],"%Y-%m-%d %H:%M:%S"), but we can leave that parsing for the parent class, if needed.

Could you point to where in the code? If it is validtime from say Mesan where get_multipoint expects a string parameter, I don't think it's a good idea. Do you mean the index in the dataframes? Isn't that a date index already?

@docNord
Copy link
Collaborator

docNord commented Apr 4, 2024

I mean in the data.period from and to fields :)

@mgcth
Copy link
Collaborator Author

mgcth commented Apr 5, 2024

Let's think about it, as Strang has those fields too.

@mgcth mgcth merged commit 55f433c into main Apr 5, 2024
12 checks passed
@mgcth mgcth deleted the feature/0.2.0 branch April 5, 2024 18:31
This was referenced Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants